Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🐛 Fixes for onboarding initiation if already logged in, UI loading in the background #1188

Merged
merged 2 commits into from
Oct 26, 2024

Conversation

JGreenlee
Copy link
Collaborator

@JGreenlee JGreenlee commented Oct 25, 2024

prevent onboarding initiation if already logged in

Bug:
If already logged in and an nrelopenpath:// link is launched (either via "JOIN" button or external QR scan), onboarding will re-initiate.
Storage from the first login is not cleared before attempting a second login and that could cause major issues.

Fix:
Prevent onboarding if onboarding has already been initiated. When handling an external URL launch we first check the onboarding state to see if the user has already initiated onboarding (i.e. gotten past "WELCOME"). If so, we'll popup with a message.

I also renamed handleOpenURL in the AppContext. cordova-plugin-customurlscheme requires a function with this name on the window object, which is why I named it that before. However we use that function to handle raw tokens too; not just URLs, so it is a misnomer.
Renamed it to handleTokenOrUrl; then defined window.handleOpenURL as a function that calls handleTokenOrUrl with a joinMethod of 'external'. This allows customurlscheme to work while keeping our code descriptive + readable.


prevent UI loading in background; refactor useAppState

    // On iOS, the UI can run if the app is launched by the OS in response to a notification,
    // in which case the appState will be 'background'. In this case, we definitely do not want
    // to load the UI because it is not visible.

useAppStateChange only accepted a callback for onResume. Refactored this into a more versatile hook useAppState, which still allows optional onResume or onChange callbacks and also returns the underlying appState.

This allows App.tsx to useAppState and return null if appState is not 'active'. in this case the App.tsx itself will still initialize but it will not have any children

TimelineContext will no longer be rendered while the app is in the background so it no longer needs to listen for app state changes.
usePermissionStatus will still listen for onResume via the new interface, since it is invoked in App.tsx.

Bug:
If already logged in and an nrelopenpath:// link is launched (either via "JOIN" button or external QR scan), onboarding will re-initiate.
Storage from the first login is not cleared before attempting a second login and that could cause major issues.

Fix:
Prevent onboarding if onboarding has already been initiated. When handling an external URL launch we first check the onboarding state to see if the user has already initiated onboarding (i.e. gotten past "WELCOME"). If so, we'll popup with a message.

I also renamed `handleOpenURL` in the AppContext. cordova-plugin-customurlscheme requires a function with this name on the window object, which is why I named it that before. However we use that function to handle raw tokens too; not just URLs, so it is a misnomer.
Renamed it to `handleTokenOrUrl`; then defined window.handleOpenURL as a function that calls `handleTokenOrUrl` with a joinMethod of 'external'. This allows customurlscheme to work while keeping our code descriptive + readable.
```
    // On iOS, the UI can run if the app is launched by the OS in response to a notification,
    // in which case the appState will be 'background'. In this case, we definitely do not want
    // to load the UI because it is not visible.
```

`useAppStateChange` only accepted a callback for onResume. Refactored this into a more versatile hook `useAppState`, which still allows optional onResume or onChange callbacks and also returns the underlying appState.

This allows App.tsx to useAppState and return null if appState is not 'active'. in this case the App.tsx itself will still initialize but it will not have any children

TimelineContext will no longer be rendered while the app is in the background so it no longer needs to listen for app state changes.
usePermissionStatus will still listen for onResume via the new interface, since it is invoked in App.tsx.
@JGreenlee JGreenlee changed the title Fixes for onboarding initiation if already logged in, UI loading in the background 🐛 Fixes for onboarding initiation if already logged in, UI loading in the background Oct 25, 2024
@JGreenlee
Copy link
Collaborator Author

Tests are consistently failing in the workflow but passing locally. It is not due to this PR; I will have to look into it separately

@shankari
Copy link
Contributor

shankari commented Oct 26, 2024

The second change resolves e-mission/e-mission-docs#1095 (comment)


useEffect(() => {
const subscription = AppState.addEventListener('change', (nextAppState) => {
if (appState.current != 'active' && nextAppState === 'active') {
onResume();
addStatReading('app_state_change', nextAppState);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great! This should finally allow us to distinguish between the app restarting and the user actually launching the app into the foreground.

@shankari shankari merged commit cfa709f into e-mission:master Oct 26, 2024
5 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Tasks completed
Development

Successfully merging this pull request may close these issues.

2 participants